Skip to content

Fix bug with ica.plot_properties#13885

Merged
drammock merged 16 commits intomne-tools:mainfrom
larsoner:ica
May 6, 2026
Merged

Fix bug with ica.plot_properties#13885
drammock merged 16 commits intomne-tools:mainfrom
larsoner:ica

Conversation

@larsoner
Copy link
Copy Markdown
Member

@larsoner larsoner commented May 5, 2026

A start toward #13879. But there are some oddities in the code... I think some parts should be plotting the variance of the dropped epochs, but silently don't currently. Now they explicitly plot zero, which is not ideal (I guess it could mean "unknown"?). The indexing logic was also broken (off-by-one error).

I suspect these plots have been wrong for some time in hard-to-detect ways. Will need to look further...

Closes #13879

@larsoner larsoner added the backport-candidate on-merge: backport to maint/1.12 label May 5, 2026
@larsoner larsoner marked this pull request as draft May 5, 2026 15:07
@larsoner larsoner marked this pull request as ready for review May 6, 2026 01:42
@larsoner
Copy link
Copy Markdown
Member Author

larsoner commented May 6, 2026

Ready for review/merge from my end!

The only example that has changed is this one because I added a reject param to show the behavior in this PR, namely that the variances are correctly shown for the rejected epochs. Notice that in the lower right variance plot the x axis (segments) spans 0 to 29 (30 segements):

image

If I run this modified example on main, you see it only goes 0-27 (28 segments), and the wrong variances are highlighted, and the perentage dropped is wrong (the calculation was 2/29 rather than 2/30):

image

larsoner added 2 commits May 6, 2026 10:06
* upstream/main:
  ENH: Add overrides= parameter to read_raw_brainvision for BIDS header repair (mne-tools#13884)
@drammock drammock enabled auto-merge (squash) May 6, 2026 14:15
@drammock drammock merged commit 893b784 into mne-tools:main May 6, 2026
31 checks passed
@drammock drammock deleted the ica branch May 6, 2026 15:21
larsoner added a commit that referenced this pull request May 6, 2026
…ties) (#13888)

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-candidate on-merge: backport to maint/1.12

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ica.plot_properties(reject='auto') raises IndexError when fit-time reject dropped any segments

2 participants